src/VCS/Adapter/Git/Gitea.php (3)

208-242: Missing blank line between listRepositoryContents closing brace and deleteRepository.

Line 242 (}) and line 243 (public function deleteRepository) are not separated by a blank line, unlike the rest of the file's method spacing.

Formatting fix

         return $contents;
     }
+
     public function deleteRepository(string $owner, string $repositoryName): bool

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/Gitea.php` around lines 208 - 242, Add a single blank
line between the closing brace of the method listRepositoryContents and the next
method declaration deleteRepository to match the file's method spacing
conventions; locate the end of listRepositoryContents() and insert one empty
line before the public function deleteRepository(...) declaration so the two
methods are separated consistently.

---

192-199: base64_decode returns false on failure — result is not validated.

If the API returns a malformed base64 string, base64_decode returns false, which would be silently placed into the content field. The GitHub adapter has the same pattern, but a defensive check avoids returning corrupt data.

Optional: guard against decode failure

         if ($encoding === 'base64') {
-            $content = base64_decode($response['body']['content'] ?? '');
+            $decoded = base64_decode($response['body']['content'] ?? '', true);
+            if ($decoded === false) {
+                throw new FileNotFound();
+            }
+            $content = $decoded;
         } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/Gitea.php` around lines 192 - 199, The code in Gitea.php
uses base64_decode on $response['body']['content'] without validating the
result, so a malformed base64 string could yield false and be returned as
content; update the handling in the method that sets $encoding/$content to check
the return of base64_decode($response['body']['content'] ?? '') and if it ===
false (or empty when non-empty input expected) throw FileNotFound (same failure
semantics as the else branch) or otherwise ensure $content is a valid string
before returning, mirroring the defensive check used in the GitHub adapter;
reference the $encoding variable, $content variable and the base64_decode call
when making this change.

---

166-177: No status-code check before consuming $response['body'] as a language map.

If the API returns a non-200 response (e.g., 404 for a deleted repo, or 500), the body will likely contain an error object with keys like message/url, and array_keys() will return those keys as "languages." The GitHub adapter has the same gap, so this is a pre-existing pattern, but it's worth noting.

Optional: guard with a status-code check

         $response = $this->call(self::METHOD_GET, $url, ['Authorization' => "token $this->accessToken"]);
 
-        if (isset($response['body'])) {
+        if (($response['headers']['status-code'] ?? 0) === 200 && isset($response['body'])) {
             return array_keys($response['body']);
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/Gitea.php` around lines 166 - 177, The
listRepositoryLanguages method reads $response['body'] without validating the
HTTP status, which can turn error objects into fake "languages"; update
listRepositoryLanguages to first check the response status (e.g., ensure
$response['status'] === 200 or a successful 2xx) before calling array_keys on
$response['body'], and return an empty array (or handle the error) when the
status is not successful; apply the same status-guard pattern where other
adapters (e.g., GitHub adapter) read $response['body'] as a language map.

tests/VCS/Adapter/GiteaTest.php (2)

79-79: Unused $response variable (flagged by PHPMD).

$response is assigned but never read. Either remove the assignment or, better yet, use it in the error message for debuggability.

Option: use $response in the error message

-        $response = curl_exec($ch);
+        $responseBody = curl_exec($ch);
         $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
         curl_close($ch);
 
         if ($httpCode >= 400) {
-            throw new \Exception("Failed to create file {$filepath}: HTTP {$httpCode}");
+            throw new \Exception("Failed to create file {$filepath}: HTTP {$httpCode} - {$responseBody}");
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` at line 79, The test assigns $response =
curl_exec($ch) but never uses it; update the code to either remove the
assignment or, preferably, include $response in the failure/error handling so
PHPMD is satisfied and debugging is easier: after calling curl_exec($ch) capture
and check the result and on error (or when curl_errno($ch) !== 0) include the
$response (and/or curl_error($ch)) in the thrown message or assertion so the
variable is consumed and the log/exception contains the HTTP response body and
error details.

---

489-489: sleep(2) may be flaky in slow CI environments.

Gitea's language detection is asynchronous, so the delay is necessary. However, a fixed 2-second sleep may not be enough under load. Consider a retry loop with a short sleep, or increase the timeout with a comment explaining why.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` at line 489, In
tests/VCS/Adapter/GiteaTest.php replace the brittle sleep(2) call with a small
retry/poll loop that checks for the expected post-processing condition (e.g.
language detection result) until either the condition is true or a reasonable
timeout elapses; specifically, in place of sleep(2) implement a loop that calls
the same check/assert you later perform (polling every 200–500ms with usleep or
sleep(1)) and break when the languages/metadata are available, or fail after a
max timeout (e.g. 10–30s), and add a short comment near the GiteaTest sleep(2)
location explaining that Gitea's language detection is asynchronous so we must
poll with a timeout to avoid CI flakiness.

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

src/VCS/Adapter/Git/Gitea.php (1)

13-15: CONTENTS_FILE / CONTENTS_DIRECTORY constants are duplicated with GitHub.php

Both adapters define identical constants. Moving them to the parent Git abstract class (or a shared interface) would give a single source of truth and avoid them drifting out of sync.

♻️ Proposed move to `src/VCS/Adapter/Git.php` (or the abstract class)

In Git.php (the abstract parent):

+    public const CONTENTS_FILE = 'file';
+    public const CONTENTS_DIRECTORY = 'dir';

Then remove the duplicate declarations from both GitHub.php and Gitea.php.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/Gitea.php` around lines 13 - 15, The CONTENTS_FILE and
CONTENTS_DIRECTORY constants are duplicated in Gitea.php and GitHub.php; move
these constants to the shared abstract Git class (e.g., add public const
CONTENTS_FILE = 'file' and public const CONTENTS_DIRECTORY = 'dir' into the Git
abstract class) and remove the duplicate declarations from Gitea.php and
GitHub.php so both adapters inherit the single source of truth (update any
references to self::CONTENTS_FILE or static::CONTENTS_DIRECTORY as needed).

tests/VCS/Adapter/GiteaTest.php (1)

492-492: sleep(2) is a fragile timing assumption for language detection

A fixed 2-second pause may still race on slow CI environments and wastes time on fast ones. Consider a short polling loop that retries until the language list is non-empty (up to a timeout), or document the minimum Gitea version/configuration that guarantees detection finishes within 2 s.

♻️ Example polling approach

-        sleep(2);
-
-        $languages = $this->vcsAdapter->listRepositoryLanguages(self::$owner, $repositoryName);
+        $languages = [];
+        $deadline = time() + 10; // 10-second timeout
+        while (empty($languages) && time() < $deadline) {
+            sleep(1);
+            $languages = $this->vcsAdapter->listRepositoryLanguages(self::$owner, $repositoryName);
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` at line 492, Replace the fragile sleep(2) in
the GiteaTest class with a short polling loop that repeatedly checks the
repository language list until it becomes non-empty or a timeout elapses; e.g.
poll the method/variable that retrieves the language list (used immediately
after sleep(2) in GiteaTest) with small sleeps (e.g. 100–200ms) up to a
configurable timeout (e.g. 5–10s) and fail the test if the list is still empty
after the timeout to avoid race conditions and wasted waits.

tests/VCS/Adapter/GiteaTest.php (2)

143-164: Consider extracting a createBranch helper.

This inline curl block mirrors the pattern in createFile. If branch creation is needed in future tests, extracting a createBranch(string $owner, string $repo, string $newBranch, string $oldBranch) helper would reduce duplication and keep test methods focused on assertions.

♻️ Proposed helper

+    private function createBranch(string $owner, string $repo, string $newBranch, string $oldBranch = 'main'): void
+    {
+        $giteaUrl = System::getEnv('TESTS_GITEA_URL', 'http://gitea:3000') ?? '';
+        $url = "{$giteaUrl}/api/v1/repos/{$owner}/{$repo}/branches";
+
+        $ch = curl_init($url);
+        curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
+        curl_setopt($ch, CURLOPT_POST, true);
+        curl_setopt($ch, CURLOPT_HTTPHEADER, [
+            'Authorization: token ' . self::$accessToken,
+            'Content-Type: application/json'
+        ]);
+        curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode([
+            'new_branch_name' => $newBranch,
+            'old_branch_name' => $oldBranch
+        ]));
+
+        $response = curl_exec($ch);
+        $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
+        curl_close($ch);
+
+        if ($httpCode >= 400) {
+            throw new \Exception("Failed to create branch {$newBranch}: HTTP {$httpCode} - {$response}");
+        }
+    }

Then the test simplifies to:

-        $giteaUrl = System::getEnv('TESTS_GITEA_URL', 'http://gitea:3000') ?? '';
-        $url = "{$giteaUrl}/api/v1/repos/" . self::$owner . "/{$repositoryName}/branches";
-
-        $ch = curl_init($url);
-        curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
-        curl_setopt($ch, CURLOPT_POST, true);
-        curl_setopt($ch, CURLOPT_HTTPHEADER, [
-            'Authorization: token ' . self::$accessToken,
-            'Content-Type: application/json'
-        ]);
-        curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode([
-            'new_branch_name' => 'feature/test-branch',
-            'old_branch_name' => 'main'
-        ]));
-
-        $response = curl_exec($ch);
-        $httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
-        curl_close($ch);
-
-        if ($httpCode >= 400) {
-            throw new \Exception("Failed to create branch: HTTP {$httpCode} - {$response}");
-        }
+        $this->createBranch(self::$owner, $repositoryName, 'feature/test-branch');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` around lines 143 - 164, The curl-based
branch creation code in GiteaTest.php duplicates logic from createFile; extract
it into a reusable helper method createBranch(string $owner, string $repo,
string $newBranch, string $oldBranch) on the GiteaTest class (or a shared test
utility) that builds the URL (using System::getEnv and
self::$owner/self::$accessToken), sets the same curl options and POST payload
('new_branch_name' and 'old_branch_name'), checks the HTTP status and throws the
same exception on >=400, then replace the inline block in the test with a call
to createBranch(self::$owner, $repositoryName, 'feature/test-branch', 'main').

---

492-492: Hardcoded sleep(2) may cause flakiness under load.

Gitea's language detection is asynchronous and timing depends on server load. A fixed 2-second sleep may be too short in CI or too long locally. A polling loop with a short sleep and a reasonable timeout would be more resilient:

♻️ Retry-based alternative

-        sleep(2);
-
-        $languages = $this->vcsAdapter->listRepositoryLanguages(self::$owner, $repositoryName);
+        $languages = [];
+        for ($i = 0; $i < 10; $i++) {
+            $languages = $this->vcsAdapter->listRepositoryLanguages(self::$owner, $repositoryName);
+            if (!empty($languages)) {
+                break;
+            }
+            usleep(500_000); // 500ms
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` at line 492, Replace the fragile sleep(2) in
the GiteaTest class with a short polling loop that repeatedly checks for the
expected language-detection outcome until a deadline; specifically, in the test
method where sleep(2) appears (class GiteaTest), implement a retry loop that
records a deadline (e.g., start + 10 seconds), then in a while loop call the
same check used later in the test (e.g., fetch the repository/file metadata or
language field via the test client) and break when the expected language is
present, otherwise usleep(200000) between attempts, and fail the test with a
clear message if the deadline is exceeded—this removes the hardcoded pause and
makes the test resilient to variable Gitea processing time.

tests/VCS/Adapter/GiteaTest.php (1)

440-440: Consider documenting why sleep(2) is needed.

The 2-second delay is presumably for Gitea's language detection to process new files. Adding a brief comment would help future maintainers understand this isn't arbitrary.

+        // Allow time for Gitea to analyze repository languages
         sleep(2);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/VCS/Adapter/GiteaTest.php` at line 440, Add a short inline comment next
to the sleep(2) call in the GiteaTest class explaining that the 2-second delay
is required to allow Gitea's language-detection/indexing to process newly
created files (so tests relying on detected languages don't race); optionally
replace the magic number with a named constant like LANGUAGE_DETECTION_DELAY or
a helper method waitForGiteaLanguageDetection() if you want clearer intent.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/VCS/Adapter/Git/GitHub.php (1)

97-124: Stub implementations satisfy the abstract contract.

The static analysis warnings about unused parameters are expected here since these are placeholder implementations. The stubs correctly throw exceptions to indicate the methods are not yet functional.

Consider using \BadMethodCallException or a custom NotImplementedException for more semantic clarity, though this is optional.

,

🔧 Optional: Use more specific exception type

 public function createFile(string $owner, string $repositoryName, string $filepath, string $content, string $message = 'Add file'): array
 {
-    throw new Exception("Not implemented");
+    throw new \BadMethodCallException("createFile is not implemented for GitHub adapter");
 }

 ...

 public function createBranch(string $owner, string $repositoryName, string $newBranchName, string $oldBranchName): array
 {
-    throw new Exception("Not implemented");
+    throw new \BadMethodCallException("createBranch is not implemented for GitHub adapter");
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/VCS/Adapter/Git/GitHub.php` around lines 97 - 124, The current stub
methods createFile and createBranch in class GitHub throw a generic Exception;
replace those throws with a more specific exception type (e.g.,
\BadMethodCallException or a project-specific NotImplementedException) to convey
intent and satisfy static analysis/semantics—update the throw expressions inside
the createFile and createBranch methods to instantiate and throw the chosen
specific exception with a clear message like "Method not implemented" so callers
and logs get better diagnostics.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro